-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
resolvergen: use the resolver type as base name for dependent types #728
resolvergen: use the resolver type as base name for dependent types #728
Conversation
Can you please add a test case demonstrating the issue? It looks like none of the generated code has changed from to your update. |
8fa2e5b
to
24b40dc
Compare
@mathewbyrne I think that may be related to the fact that the
|
The template was outputing invalid code since the resolver type was not used in places like the embedding at {query,mutation}Resolver. This change also ensures that objects like {query,mutation}Resolver also use the user provided type name as suffix. Here's the resulting diff on the code generation with `type: GeneratedResolver` in the resolver config: ``` diff -u resolver.go resolvernew.go --- resolver.go 2019-05-26 20:04:15.361969755 -0300 +++ resolvernew.go 2019-05-26 20:04:54.170737786 -0300 @@ -7,20 +7,20 @@ type GeneratedResolver struct{} func (r *GeneratedResolver) Mutation() MutationResolver { - return &mutationResolver{r} + return &mutationGeneratedResolver{r} } func (r *GeneratedResolver) Query() QueryResolver { - return &queryResolver{r} + return &queryGeneratedResolver{r} } -type mutationResolver struct{ *Resolver } +type mutationGeneratedResolver struct{ *GeneratedResolver } -func (r *mutationResolver) CreateTodo(ctx context.Context, input NewTodo) (*Todo, error) { +func (r *mutationGeneratedResolver) CreateTodo(ctx context.Context, input NewTodo) (*Todo, error) { panic("not implemented") } -type queryResolver struct{ *Resolver } +type queryGeneratedResolver struct{ *GeneratedResolver } -func (r *queryResolver) Todos(ctx context.Context) ([]*Todo, error) { +func (r *queryGeneratedResolver) Todos(ctx context.Context) ([]*Todo, error) { panic("not implemented") } ```
24b40dc
to
c5acbea
Compare
} | ||
|
||
t.Run("renders expected contents", func(t *testing.T) { | ||
m := &Plugin{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe an easier and simpler way to write this test would be to use golden files (https://medium.com/@jarifibrahim/golden-files-why-you-should-use-them-47087ec994bf)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already do this using code review of the committed generated files. No need for update, just commit the generated code and push.
All this test should need to do is call the generator with some config, eg. It may not look like its asserting anything, but really its output is tested in three ways:
- firstly we run goimports when saving go files, so running generate should error out in the test if it generates invalid code.
- if that fails the linter should see invalid code, and fail the branch.
- code review catches changes. diffs are readable in github.
@@ -19,6 +19,7 @@ type Plugin struct{} | |||
var _ plugin.CodeGenerator = &Plugin{} | |||
|
|||
func (m *Plugin) Name() string { | |||
// TODO: typo, should be resolvergen |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😆
} | ||
|
||
t.Run("renders expected contents", func(t *testing.T) { | ||
m := &Plugin{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already do this using code review of the committed generated files. No need for update, just commit the generated code and push.
All this test should need to do is call the generator with some config, eg. It may not look like its asserting anything, but really its output is tested in three ways:
- firstly we run goimports when saving go files, so running generate should error out in the test if it generates invalid code.
- if that fails the linter should see invalid code, and fail the branch.
- code review catches changes. diffs are readable in github.
…r-dependent-types-follow-configured-type resolvergen: use the resolver type as base name for dependent types
The template was outputing invalid code since the resolver type was
not used in places like the embedding at {query,mutation}Resolver.
This change also ensures that objects like {query,mutation}Resolver
also use the user provided type name as suffix.
Here's the resulting diff on the code generation with
type: GeneratedResolver
in the resolver config:I haven't yet found the place where this is tested nor the sensible way to add test for this use case. Will update as soon as I find it.
I have: